Skip to content

Conversation

snskArora
Copy link

This would resolve #405

@snskArora snskArora requested review from a team, ayushmjain and q2w as code owners June 21, 2025 09:58
@snskArora snskArora force-pushed the feature/terminal_autoclass branch from 7d00f3b to 2d7b4ef Compare June 21, 2025 10:00
@snskArora snskArora changed the title Adding terminal autoclass variable for simple and multiple bucket feat: Adding terminal autoclass variable for simple and multiple bucket Jun 21, 2025
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @snskArora

variables.tf Outdated
type = map(string)
default = {}
validation {
condition = alltrue([for _, v in var.terminal_autoclass : var.autoclass == false || v == "NEARLINE" || v == "ARCHIVE"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we maintain a min TF compat with tf 1.3 we cannot use cross variable reference in validation rules (i.e checking var.autoclass) which was introduced in 1.9. If there is a validation error if this is set and autoclass is disabled could we perform a check before passing value to terminal_storage_class - something like lookup(var.autoclass..) ? lookup(var.terminal_autoclass...) : null

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bharathkkb

The lookup during the assignment would be of no use as this is specifically to root out the wrong input (basically converting the string to an enum). Since it has to be compatible with v1.3 I will remove the autoclass check from the validation block.

@snskArora snskArora requested a review from bharathkkb August 17, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants